Skip to content

Add support for Directory AccessControl#261

Merged
tathamoddie merged 1 commit intoTestableIO:masterfrom
russcam:fix/directory-access-control
Nov 16, 2017
Merged

Add support for Directory AccessControl#261
tathamoddie merged 1 commit intoTestableIO:masterfrom
russcam:fix/directory-access-control

Conversation

@russcam
Copy link
Copy Markdown
Contributor

@russcam russcam commented Nov 16, 2017

Supercedes #259 (meant to branch for the PR as opposed to creating on master).

Add support for both getting and setting AccessControl on directories.

It looks like the implementation of AddDirectory(string path) on MockFileSystem should also accept a MockDirectoryData parameter, similar to how AddFile(string path, MockFileData mockFile) accepts MockFileData. This PR doesn't include that change

@tathamoddie tathamoddie merged commit d8fef25 into TestableIO:master Nov 16, 2017
@tathamoddie
Copy link
Copy Markdown
Contributor

Thanks! Shipped to NuGet in 2.1.0.175

@russcam
Copy link
Copy Markdown
Contributor Author

russcam commented Nov 16, 2017

Thanks!

@russcam
Copy link
Copy Markdown
Contributor Author

russcam commented Nov 20, 2017

It looks like the implementation of AddDirectory(string path) on MockFileSystem should also accept a MockDirectoryData parameter, similar to how AddFile(string path, MockFileData mockFile) accepts MockFileData.

What are your thoughts on ^ @tathamoddie? Happy to submit a PR for this change if it makes sense (guess you'd bump the major if adhering to SemVer)?

@tathamoddie
Copy link
Copy Markdown
Contributor

Could it be added as an optional overload?

For AddFile, we always need the file data to know what's in it.

For AddDirectory, we'd only need a MockDirectory object if you want to init it with special access control restrictions. All existing test code could continue to default as-is.

@russcam
Copy link
Copy Markdown
Contributor Author

russcam commented Nov 23, 2017

I can add it as an overload on the constructor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants